Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: be936b6edefb8d319b6693eb1f36230b716377da (build)

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ca8f5dd:

Perf Analysis (@fluentui/react-components)

No significant results to display.

📊 Bundle size report

Perf Analysis (@fluentui/react-northstar)

⚠️ No perf measurements available

🕵 fluentuiv9 No visual regressions between this PR and main

Perf Analysis (@fluentui/react)

No significant results to display.

Had a few hiccups with this as it increased bundle size for the v9 packages. Finally got the correct setup for the .swcrc files which decreases (which is what we expect) the bundle size. See results here: #27036 (comment)

🕵 fluentuiv8 No visual regressions between this PR and main

Ran npm diff --diff=@fluentui/[email protected] --diff=@fluentui/[email protected] ./lib/ --json > output.json to see any possible build output differences between the last nightly release based on master (@fluentui/[email protected]) and the nightly release based on my test branch with swc transpilation (@fluentui/[email protected]) and the only things that really stood out (amid all the noise) are the difference in helper function imports (from ts to swc), the paths to sources and some more liberated use of modern syntax like nullish coalescing which our browser matrix supports.

@TristanWatanabe there is a difference in the diff that no one noticed:

Considering an amount of object spreads it's a bummer that it gets transformed.

I checked that we should always fall into the path with Object.assign (https://github.com/swc-project/swc/blob/e0d4220264ec3926f0a5d6b774ea782f2e511a08/packages/swc-helpers/src/_extends.mjs#L2), but additional function calls are newer free. Especially, it's strange for this feature as object spread is few years older than nullish coalescing and should work natively everywhere in our browser matrix.

EDIT: Looks like bf390c0 fixed that, so ignore it 🐱

Ran npm diff --diff=@fluentui/[email protected] --diff=@fluentui/[email protected] ./lib/ --json > output.json to see any possible build output differences between the last nightly release based on master (@fluentui/[email protected]) and the nightly release based on my test branch with swc transpilation (@fluentui/[email protected]) and the only things that really stood out (amid all the noise) are the difference in helper function imports (from ts to swc), the paths to sources and some more liberated use of modern syntax like nullish coalescing which our browser matrix supports.

@TristanWatanabe there is a difference in the diff that no one noticed:

Considering an amount of object spreads it's a bummer that it gets transformed.

I checked that we should always fall into the path with Object.assign (swc-project/swc@e0d4220/packages/swc-helpers/src/_extends.mjs#L2), but additional function calls are newer free. Especially, it's strange for this feature as object spread is few years older than nullish coalescing and should work natively everywhere in our browser matrix.

EDIT: Looks like bf390c0 fixed that, so ignore it 🐱

@layershifter Yup we caught that when the bundle size ballooned due to the helper function being used for the spread. @spmonahan actually filed an issue to swc regarding the object spread: swc-project/swc#7044

🕵 FluentUI-v0 No visual regressions between this PR and main

@layershifter Yup we caught that when the bundle size ballooned due to the helper function being used for the spread. @spmonahan actually filed an issue to swc regarding the object spread: swc-project/swc#7044

@TristanWatanabe Looks like the PR for the issue was closed so I'm not sure if a fix is imminent. However, in looking at the PR it seems like is a (undocumented?) useSpread option. Can you give that a try?

https://github.com/swc-project/swc/pull/7054/files#diff-8abacd70370fd7f85bb3452458bd542918f7b0a9dc6074dce36d726593c4efe0R833

@layershifter Yup we caught that when the bundle size ballooned due to the helper function being used for the spread. @spmonahan actually filed an issue to swc regarding the object spread: swc-project/swc#7044

@TristanWatanabe Looks like the PR for the issue was closed so I'm not sure if a fix is imminent. However, in looking at the PR it seems like is a (undocumented?) useSpread option. Can you give that a try?

swc-project/swc#7054 (files)

@spmonahan I just tried adding useSpread:true to the .swcrc file of react-text and now instead of using Object.assign or spreading, it just flat out passes slotProps.root 😬

 * runtime code filtering props in this function.
 *
 * @param state - State including slot definitions
 * @returns An object containing the `slots` map and `slotProps` map.
 */
        function(state) {
            const slots = {}, slotProps = {}, slotNames = Object.keys(state.components);
            for (const slotName of slotNames) {
                const [slot, props] = getSlot(state, slotName);
                slots[slotName] = slot, slotProps[slotName] = props;
            }
            return {
                slots: slots,
                slotProps: slotProps
            };
        }(state);
        /*#__PURE__*/
        return external_React_namespaceObject.createElement(slots.root, slotProps.root);
    }, __GLOBAL__ = "undefined" == typeof window ? __webpack_require__.g : window;

@spmonahan I just tried adding useSpread:true to the .swcrc file of react-text and now instead of using Object.assign or spreading, it just flat out passes slotProps.root 😬

Interesting. I wonder if maybe that's even more desirable behavior than directly using spread? Just passing the props seems to be the "default" Babel behavior.

@Hotell / @layershifter do you know why we used spread with the old/current build tooling?

@Hotell / @layershifter do you know why we used spread with the old/current build tooling?

That's interesting:

// INPUT
import * as React from 'react'

function App(props) {
    return <div {...props} />
}
function App2(props) {
    return <div {...props} aria-label="foo" />
}

⬇️⬇️⬇️

// OUTPUT BY TS
import * as React from 'react';
function App(props) {
    return React.createElement("div", { ...props });
}
function App2(props) {
    return React.createElement("div", { ...props, "aria-label": "foo" });
}

// OUTPUT BY BABEL
import * as React from 'react';
function App(props) {
  return /*#__PURE__*/React.createElement("div", props);
}
function App2(props) {
  return /*#__PURE__*/React.createElement("div", _extends({}, props, {
    "aria-label": "foo"
  }));
}

AFAIR it's okay to avoid spread there as React does a loop through props anyway.

@Hotell / @layershifter do you know why we used spread with the old/current build tooling?

That's interesting:

// INPUT
import * as React from 'react'

function App(props) {
    return <div {...props} />
}
function App2(props) {
    return <div {...props} aria-label="foo" />
}

⬇️⬇️⬇️

// OUTPUT BY TS
import * as React from 'react';
function App(props) {
    return React.createElement("div", { ...props });
}
function App2(props) {
    return React.createElement("div", { ...props, "aria-label": "foo" });
}

// OUTPUT BY BABEL
import * as React from 'react';
function App(props) {
  return /*#__PURE__*/React.createElement("div", props);
}
function App2(props) {
  return /*#__PURE__*/React.createElement("div", _extends({}, props, {
    "aria-label": "foo"
  }));
}

AFAIR it's okay to avoid spread there as React does a loop through props anyway.

To add on to this, the swc output with useSpread:true is basically almost identical to the TS output:

// SWC output via https://swc.rs/playground
import * as React from 'react';
function App(props) {
    return /*#__PURE__*/ React.createElement("div", props);
}
function App2(props) {
    return /*#__PURE__*/ React.createElement("div", {
        ...props,
        "aria-label": "foo"
    });
}

To add on to this, the swc output with useSpread:true is basically almost identical to the TS output:

Just so I'm clear, useSpread would work for us then?

To add on to this, the swc output with useSpread:true is basically almost identical to the TS output:

Just so I'm clear, useSpread would work for us then?

Haven't investigated it yet but it's now consistently failing for ssr-tests when I fully tested it with all v9 packages using useSpread so maybe not? https://uifabric.visualstudio.com/fabricpublic/_build/results?buildId=292722&view=logs&j=258ec178-2d8b-5611-7b9b-60c5c95dae55&t=254ee48e-1503-524f-f75e-e2c69d0380f5

To add on to this, the swc output with useSpread:true is basically almost identical to the TS output:

Just so I'm clear, useSpread would work for us then?

Haven't investigated it yet but it's now consistently failing for ssr-tests when I fully tested it with all v9 packages using useSpread so maybe not? uifabric.visualstudio.com/fabricpublic/_build/results?buildId=292722&view=logs&j=258ec178-2d8b-5611-7b9b-60c5c95dae55&t=254ee48e-1503-524f-f75e-e2c69d0380f5

Hmm I was able to get a good build this time around. I wasn't able to repro the puppeteer navigation timeout error that was popping up in CI on my local machine so it was hard to debug. The ssr-tests-v9 timeout errors may be completely unrelated to the swc transpilation change anyway, that's been a known intermittent issue for a while now that pops up on the build failures channel quite frequently 🤔. With what @layershifter said regarding not needing a spread and the swc output with useSpread:true being almost identical with TS, I'm fine with replacing useBuiltIns with useSpread instead

I'm fine with replacing useBuiltIns with useSpread instead

I posted a follow up question on my thread for the SWC folks to make sure useSpread is something safe for us to use since I'm not able to find it in the SWC docs. But otherwise, sounds good to me.

I posted a follow up question on my thread for the SWC folks to make sure useSpread is something safe for us to use since I'm not able to find it in the SWC docs.

Looks like we should use it.

Released a new nightly version (0.0.0-nightly-20230317-1454.1) to check and components worked and loaded as expected on a CRA. Will merge this in now and make PR applying these updates to all v9 packages ready for review.